ref(dynamic-sampling): Migrate projectSampling to new form system#109360
Merged
priscilawebdev merged 19 commits intomasterfrom Mar 30, 2026
Merged
ref(dynamic-sampling): Migrate projectSampling to new form system#109360priscilawebdev merged 19 commits intomasterfrom
priscilawebdev merged 19 commits intomasterfrom
Conversation
github-actions
bot
added
the
Scope: Frontend
JonasBa
changed the title
Contributor
|
This pull request has gone three weeks without activity. In another week, I will close it. But! If you comment or otherwise update it, I will reset the clock, and if you add the label "A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀 |
priscilawebdev
force-pushed
the
jb/forms/dynamic-sampling-org-sampling
branch
from
3006dd5 to
be5711c
Compare
Base automatically changed from
jb/forms/dynamic-sampling-org-sampling
to
master
March 26, 2026 08:37
…ystem Replace the custom formContext-based form in projectSampling with useScrapsForm from @sentry/scraps/form, completing the migration started for organizationSampling. - Delete projectSamplingForm.tsx and formContext.tsx — now dead code - Rewrite projectSampling.tsx using useScrapsForm with a z.record schema. A useEffect mirrors the old enableReInitialize behaviour, resetting the form whenever the server data changes. Per-project validity is checked inline in the AppField render prop (same logic as getProjectRateErrors) to keep the Apply Changes button disabled when any rate is invalid. - Rewrite projectsEditTable.tsx to receive projectRates, savedProjectRates, and onProjectRatesChange as explicit props instead of pulling from the form context via useFormField. Per-project validation errors are now computed locally via getProjectRateErrors, which keeps the validation logic co-located with the table that displays it. Co-Authored-By: Claude <noreply@anthropic.com>
priscilawebdev
force-pushed
the
jb/forms/dynamic-sampling-project-sampling
branch
from
a197f03 to
f92ccef
Compare
Use AppForm form prop, SubmitButton, canSubmit state, and remove FormWrapper to match the pattern established in organizationSampling. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Extract sampleRateField from organizationSampling and reuse it in projectSamplingSchema via z.record(). This removes the duplicate manual validation (hasProjectRateErrors, getProjectRateErrors) and lets the form system handle submit-blocking through canSubmit. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Use projectRates as defaultValues instead of an empty object so fields show their current values immediately rather than rendering empty on first paint. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Zod schema now handles per-value validation through the form system. Remove the redundant manual error computation that was running on every render regardless of submission state. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Pass the field error from the form to the table footer so users see feedback when Zod validation blocks submission. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace the footer error with per-row validation errors next to each invalid field. Errors only appear after the form has been submitted and Zod validation has flagged the field as invalid. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace manual validation checks with sampleRateField.safeParse() so per-row errors use the same Zod schema as the form-level validation. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace the single projectRates record field with per-project
AppField instances. Each row now gets its own field via
form.AppField name={projectRates.${projectId}}, so the form system
handles per-field validation, error display, and canSubmit natively.
Removes the showErrors/getProjectRateError workaround. Bulk edit
uses form.setFieldValue per project instead of updating the record.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
priscilawebdev
force-pushed
the
jb/forms/dynamic-sampling-project-sampling
branch
from
3a1b5a9 to
369a758
Compare
Move form awareness out of ProjectsTable into ProjectsEditTable. Read per-field errors from form.state.fieldMeta and pass them via the existing items array. ProjectsTable is now a pure display component again. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace the form prop with plain data props (projectRates, projectErrors, onProjectRateChange) so the edit table is a pure presentational component with no form system imports or any types. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The submit button should always be clickable so users can trigger validation and see errors. Only disable based on access permissions. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Cover initial render, edit/reset flow, validation on submit, API payload, post-save state, error handling, and access control. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
priscilawebdev
force-pushed
the
jb/forms/dynamic-sampling-project-sampling
branch
from
d7ef56d to
a4381d4
Compare
Add an accessible label to each project's sample rate input so screen readers can identify which project the input belongs to. Update the test helper to query by this label instead of relying on positional spinbutton indexing. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The bulk org rate edit was calling setFieldValue in a loop for each project, triggering N separate validations. Use a single atomic update for all project rates instead, matching the old behavior. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When there are no span counts for the selected period, the division by zero produces NaN which renders as a blank number input. Default to 0% when totalSpanCount is zero. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
priscilawebdev
approved these changes
Mar 30, 2026
Member
priscilawebdev
left a comment
priscilawebdev
left a comment
There was a problem hiding this comment.
I did manual testing, and everything looks good
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
| sampleRate: value[project.id]!, | ||
| error: error?.[project.id], | ||
| initialSampleRate: savedProjectRates[project.id]!, | ||
| sampleRate: projectRates[project.id]!, |
Contributor
There was a problem hiding this comment.
Non-null assertions on potentially undefined project rates
Low Severity
savedProjectRates[project.id]! and projectRates[project.id]! use non-null assertions but can be undefined during the transition between data loading and the useEffect resetting the form. savedProjectRates is initialized via useState(projectRates) where projectRates is {} on first render, and is only populated after the useEffect fires — one render frame after isLoading becomes false. This causes inputs to receive undefined as their value prop, triggering React's uncontrolled-to-controlled warning (acknowledged and suppressed in the test). Using ?? '' instead of ! would provide a safe fallback and eliminate the transition.
Additional Locations (1)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Completes the form migration started in the previous PR by replacing the
custom
formContext-based form inprojectSamplingwithuseScrapsForm.What changed:
projectSampling.tsx— replacesuseFormState/FormProviderwithuseScrapsForm. AuseEffecton the query data mirrors the oldenableReInitialize: truebehaviour, resetting the form (andsavedProjectRates) whenever server data arrives or changes. Per-project rate validity is checked inline in theAppFieldrender prop to disable the Apply Changes button, sincez.record(z.string(), z.string())is used for the schema (type safety only — Zod'sZodEffectschained on record values isn't assignable toFormValidateOrFn).projectsEditTable.tsx— removes theuseFormField('projectRates')context consumer and instead receivesprojectRates,savedProjectRates, andonProjectRatesChangeas explicit props. Per-project validation errors are now computed locally viagetProjectRateErrors, keeping the validation logic co-located with the table that displays it.utils/projectSamplingForm.tsx— deleted.utils/formContext.tsx— deleted; this was the shared custom form context, now fully unused.Stacks on top of #109356.